Some fixes for issues found with afl and address sanitizer (#499)

* Fix off-by-one in nmea reader

Found with afl and address sanitizer:

AddressSanitizer:DEADLYSIGNAL
=================================================================
==2615627==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f54eaa53683 bp 0x7ffd187eed00 sp 0x7ffd187ee4b8 T0)
==2615627==The signal is caused by a READ memory access.
==2615627==Hint: address points to the zero page.
    #0 0x7f54eaa53682 in QString::chop(int) (/lib64/libQt5Core.so.5+0x13c682)
    #1 0x5e8d7f in gpgsa_parse /home/gpsbabel/gpsbabel.git/nmea.cc:744
    #2 0x5e8d7f in nmea_parse_one_line /home/gpsbabel/gpsbabel.git/nmea.cc:1021
    #3 0x5f0b6f in nmea_read /home/gpsbabel/gpsbabel.git/nmea.cc:1096
    #4 0xc7e483 in run /home/gpsbabel/gpsbabel.git/main.cc:339
    #5 0x4ced15 in main /home/gpsbabel/gpsbabel.git/main.cc:707
    #6 0x7f54ea3f71a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)
    #7 0x4cffdd in _start (/home/gpsbabel/gpsbabel.git/gpsbabel+0x4cffdd)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/lib64/libQt5Core.so.5+0x13c682) in QString::chop(int)
==2615627==ABORTING

* Fix heap buffer overflow in igc reader

In gbfgetstr(), when file->buff contains exactly file->buffsz
characters including null termination, there is no room to
append another character with strcat() in igc.cc

Found by afl with address sanitizer:

==2082077==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x611000001f40 at pc 0x7f5a4e0da6cd bp 0x7ffd14ffe040 sp 0x7ffd14ffd7e8
WRITE of size 2 at 0x611000001f40 thread T0
    #0 0x7f5a4e0da6cc  (/lib64/libasan.so.5+0x9b6cc)
    #1 0x7327cf in data_read /home/gpsbabel/gpsbabel.git/igc.cc:334
    #2 0xc7a3c1 in run /home/gpsbabel/gpsbabel.git/main.cc:339
    #3 0x4cddf2 in main /home/gpsbabel/gpsbabel.git/main.cc:707
    #4 0x7f5a4d5e51a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)
    #5 0x4cf00d in _start (/home/gpsbabel/gpsbabel.git/gpsbabel+0x4cf00d)

0x611000001f40 is located 0 bytes to the right of 256-byte region [0x611000001e40,0x611000001f40)

* Fix endless loop in mapsource

The loop around gbfread needs a check for eof, otherwise it
may never terminate with special input created by afl.

diff --git a/igc.cc b/igc.cc
index 90b57b397cc0ab4a8e80d8e876adbdee1e72af65..0dbbafed3e3da4f4012dc3405a6c3c2394cd810e 100644 (file)
--- a/igc.cc
+++ b/igc.cc
@@ -331,9 +331,10 @@ static void data_read()
       } else {
         // Store other header data in the track descriptions
         if (strlen(trk_desc) < MAXDESCLEN) {
-          strcat(ibuf, HDRDELIM);
           remain = MAXDESCLEN - strlen(trk_desc);
           strncat(trk_desc, ibuf, remain);
+          remain = MAXDESCLEN - strlen(trk_desc);
+          strncat(trk_desc, HDRDELIM, remain);
         }
       }
       break;

diff --git a/mapsource.cc b/mapsource.cc
index b67535e6d7cab916c6daa03d7d8b710a73405a8c..54d4a7b41e547ba983a54dd0e285a00e8d5d301e 100644 (file)
--- a/mapsource.cc
+++ b/mapsource.cc
@@ -926,7 +926,7 @@ mps_route_r(gbfile* mps_file, int mps_ver, route_head** rte)
          data (min 22 bytes) terminated by a zero */
       do {
         gbfread(tbuf, 1, 1, mps_file);
-      } while (tbuf[0]);
+      } while (tbuf[0] && !gbfeof(mps_file));
 
       /* The next thing is the unknown 0x03 0x00 .. 0x00 (18 bytes) */
       gbfread(tbuf, 18, 1, mps_file);
@@ -1039,7 +1039,7 @@ mps_route_r(gbfile* mps_file, int mps_ver, route_head** rte)
        data (min 22 bytes) terminated by a zero */
     do {
       gbfread(tbuf, 1, 1, mps_file);
-    } while (tbuf[0]);
+    } while (tbuf[0] && !gbfeof(mps_file));
 
     /* The next thing is the unknown 0x03 0x00 .. 0x00 (18 bytes) */
     gbfread(tbuf, 18, 1, mps_file);

diff --git a/nmea.cc b/nmea.cc
index 167b0767ff4329b3b7c546eb214bbecafca2f265..6100292b3d6ff3d03b90bee49a7d8e4c7f1ce96c 100644 (file)
--- a/nmea.cc
+++ b/nmea.cc
@@ -727,7 +727,7 @@ gpgsa_parse(char* ibuf)
   // 0 = "GPGSA"
   // 1 = Mode. Ignored
   QChar fix;
-  if (nfields > 1) {
+  if (nfields > 2) {
     fix = fields[2][0];
   }
 
@@ -737,9 +737,9 @@ gpgsa_parse(char* ibuf)
   }
 
   float pdop = 0, hdop = 0, vdop = 0;
-  if (nfields > 14) pdop = fields[15].toFloat();
-  if (nfields > 15) hdop = fields[16].toFloat();
-  if (nfields > 16) {
+  if (nfields > 15) pdop = fields[15].toFloat();
+  if (nfields > 16) hdop = fields[16].toFloat();
+  if (nfields > 17) {
      // Last one is special. The checksum isn't split out above.
     fields[17].chop(3);
     vdop = fields[17].toFloat();